[CALCITE-7331] Support the alias form SELECT * EXCEPT() for SELECT * EXCLUDE() #4682
Conversation
xiedeyantu
force-pushed
the
CALCITE-7331
branch
from
0903c79 to
32255af
Compare
|
Conflicts in #4683 has been resolved. |
|
|
I'd like to wait and see if @julianhyde has any other comments. |
xiedeyantu
added
the
LGTM-will-merge-soon
|
If there are no further comments within 48 hours, I will consider merging this PR. |
xiedeyantu
added
discussion-in-jira
|
The discussion is still ongoing on Jira, and we may need to wait for @julianhyde’s input. |
|
This PR hasn’t received a response from Julian yet. I’m not sure if this will add to the parser’s burden, so should we close this PR? |
|
I hate reviewing PRs, so I'm not going to +1 this, but you don't need a +1 from me. If the specification is clear, and the code implements the specification, go ahead and merge. |
Thank you for your reply. I will add the test cases mentioned in Jira to this PR and continue to complete it. |
xiedeyantu
removed
the
discussion-in-jira
xiedeyantu
force-pushed
the
CALCITE-7331
branch
from
32255af to
16af9e2
Compare
|
I added some tests to the quidem in Babel (from a discussion on Jira), and the conflicts in the main branch of the rebase were resolved. There were no other changes. @mihaibudiu If you have time, could you please check if it's ready? |
|
| SqlIdentifier id; | ||
| } | ||
| { | ||
| <EXCLUDE> <LPAREN> { s = span(); } |
There was a problem hiding this comment.
this could potentially be configurable, but I guess that people can adapt the parser and choose what they want.
There was a problem hiding this comment.
Yes, we'll keep it simple for now, supporting both, and keeping the configuration parameters as minimal as possible.
xiedeyantu
added
the
LGTM-will-merge-soon
3 participants
See CALCITE-7331